Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding a dynamic attribute "sceneInterface:linkHash" computed by LinkedScene #19

Closed
wants to merge 2 commits into from

Conversation

ldmoser
Copy link

@ldmoser ldmoser commented Jul 4, 2013

The motivation for this new attribute is the following:

  • We want to be able to optimize the OpenGL visualization of large scenes and seems natural to use instancing at places where the scene refers to external scenes (there's a good chance that an external scene may be referred several times in the same master scene). By having a unique hash that takes in consideration the time remapping as well, we can safely instantiate the scene, knowing that, if the time changes we will still be able to apply updates as opposed to have to recreate the entire GL scene from scratch.
  • We also look for ways to, at render time, be able to know that a given sub-tree of the scene matches exactly a previously seen one, and use 3delight procedural instancing to save memory and computation resources.

Two important caveats:

  1. It computes the hash every time the attribute value is requested. No caches. Which may impact performance if used naively. The assumption is that readers would need that information only the first time, then (when time changes), they have already the knowledge of what's unique and will not require to load the attribute again. It was done that way for the sake of simplicity of implementation and should be re-evaluated if performance really becomes a problem.

  2. It does not return the attribute by the function attributeNames(). I've opted to not add because on normal circumstances, that attribute is not important for generic readers. Only readers that know about it, will pay the price to compute it's value. Maybe if we find a way to bake the hash in the scene, then we could change that with no bad consequences.

@andrewkaufman
Copy link
Member

John, what do you think about saving hashes as StringData? We didn't really want to add MurmurHashData or some new method on the SceneInterface...

@ldmoser
Copy link
Author

ldmoser commented Jul 5, 2013

Just a clarification.. there's currently no storing of hashes in the file. They are computed dynamically. But yeah, if we had MurmurHashData, then I guess I would return that. But I figured it would be not really detrimental, as the final hash is not supposed to be stored on disk. But it's supposed to be something that is unique for a given master LinkedScene, in the context of reading it. So nothing prevent us from returning later a MurmurHashData without breaking compatibility.

You are right about the tests, since they hard code the hashed to compare against. Besides being weak, it may indicate we want to enforce the hashes to not change over time, which is not true. I will change the tests.

@johnhaddon
Copy link
Member

Since we're not storing them, using StringData for now seems alright to me - I'm not particularly keen on introducing new Data types unless there's a clear benefit.

@andrewkaufman
Copy link
Member

Lucio wants to hold off on merging this until he investigates some other ideas.

Lucio Moser added 2 commits July 12, 2013 09:42
…edScene whenever there's a link to an external file in the hierarchy loaded. The hash is unique as to time remapping and so can be used safely for instancing when reading.
…on specific hash values as they can change over time, but the relations are important.
@andrewkaufman
Copy link
Member

Closing for now, in favour of something like #26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants